Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

COMPAT: Update for NumPy dev #17987

Merged
merged 1 commit into from
Oct 27, 2017
Merged

Conversation

TomAugspurger
Copy link
Contributor

Closes #17986

xref numpy/numpy#9487

Do we want this for 0.21?

@TomAugspurger TomAugspurger added the Compat pandas objects compatability with Numpy or Python functions label Oct 25, 2017
return np.fromstring(values, dtype=dtype)
# Copy the bytes into a numpy array.
buf = np.frombuffer(values, dtype=dtype)
buf = buf.copy() # required to not mutate the original data
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's safe to not copy here, IIRC values is pile of bytes read from a msgpack file, so no risk in mutating?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without the copy the test failed:

        for buf, control_buf in zip(not_garbage, control):
            # make sure none of our mutations above affected the
            # original buffers
>           assert buf == control_buf
E           AssertionError: assert b'\x00\x00\x0...x00\x00@\x8f@' == bytearray(b'\x...00\x008\x8f@')
E             At index 6 diff: 240 != 0
E             Use -v to get the full diff

pandas/tests/io/test_packers.py:690: AssertionError

I didn't look closely to see if that's a legitimate problem or not.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, in that case I would trust the test.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you need to do this conditionally for numpy > 1.13 I think

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests on the older NumPy passed, let me make sure zlib is installed and that they weren't skipped.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty sure that https://travis-ci.org/pandas-dev/pandas/jobs/292845756 hit it, so I think we're OK.

@codecov
Copy link

codecov bot commented Oct 25, 2017

Codecov Report

Merging #17987 into master will increase coverage by 0.33%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #17987      +/-   ##
==========================================
+ Coverage   91.23%   91.57%   +0.33%     
==========================================
  Files         163      163              
  Lines       50113    53390    +3277     
==========================================
+ Hits        45723    48892    +3169     
- Misses       4390     4498     +108
Flag Coverage Δ
#multiple 89.52% <100%> (+0.48%) ⬆️
#single 41.01% <0%> (+0.64%) ⬆️
Impacted Files Coverage Δ
pandas/io/packers.py 88.3% <100%> (+0.1%) ⬆️
pandas/io/gbq.py 25% <0%> (-58.34%) ⬇️
pandas/core/categorical.py 95.44% <0%> (-0.29%) ⬇️
pandas/core/panel4d.py 100% <0%> (ø) ⬆️
pandas/core/internals.py 94.77% <0%> (+0.31%) ⬆️
pandas/core/panel.py 97.39% <0%> (+0.43%) ⬆️
pandas/core/generic.py 93.06% <0%> (+0.52%) ⬆️
pandas/core/frame.py 98.54% <0%> (+0.69%) ⬆️
pandas/core/ops.py 92.99% <0%> (+1.21%) ⬆️
pandas/core/reshape/concat.py 98.84% <0%> (+1.26%) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 36c309e...cb0101b. Read the comment docs.

@TomAugspurger TomAugspurger added this to the 0.21.1 milestone Oct 26, 2017
@TomAugspurger TomAugspurger deleted the np-dev-warnings branch October 27, 2017 12:04
@TomAugspurger TomAugspurger restored the np-dev-warnings branch October 27, 2017 12:04
@TomAugspurger TomAugspurger reopened this Oct 27, 2017
@TomAugspurger
Copy link
Contributor Author

Huh, must have deleted that branch accidentally.

@TomAugspurger TomAugspurger mentioned this pull request Oct 27, 2017
64 tasks
@jreback jreback merged commit dff5109 into pandas-dev:master Oct 27, 2017
@jreback
Copy link
Contributor

jreback commented Oct 27, 2017

thanks!

peterpanmj pushed a commit to peterpanmj/pandas that referenced this pull request Oct 31, 2017
No-Stream pushed a commit to No-Stream/pandas that referenced this pull request Nov 28, 2017
@TomAugspurger TomAugspurger deleted the np-dev-warnings branch December 8, 2017 18:26
TomAugspurger added a commit to TomAugspurger/pandas that referenced this pull request Dec 8, 2017
TomAugspurger added a commit that referenced this pull request Dec 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Compat pandas objects compatability with Numpy or Python functions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants